Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VACMS 15823 - Provider Core Training icon and wording #30734

Merged
merged 9 commits into from
Jul 11, 2024

Conversation

eselkin
Copy link
Contributor

@eselkin eselkin commented Jul 8, 2024

Summary

  • If trainings on provider has trainings from PPMS then displays icon and "Provider core training" wording. Only added to CCProviderResult.jsx, UrgentCareResult.jsx, and EmergencyCareResult.jsx result items types.
  • This is the solution because we did not display data related to trainings before.
  • Sitewide Facilities

Related issue(s)

Testing done

  • Tested manually since training data is still incredibly sparse
  • Added automated testing for rendering new component

Screenshots

Shows a provider with and without training Screenshot 2024-07-09 at 3 31 56 PM

What areas of the site does it impact?

Facility Locator

Acceptance criteria

  • Recommend the label reads "Provider core training" (no "complete"), no link out
  • When facility type = "Community providers in VA's network", community providers with training data display the training icon and label.
  • When facility type = "Urgent care" and service type = "All in network urgent care" or "In network community urgent care", community providers with training data display the training icon and label.
  • When facility type = "Emergency care" and service type = "All in network emergency care" or "In network community emergency care", community providers with training data display the training icon and label.
  • Icon and label display consistent with design
  • icon uses aria-hidden = true
  • Only providers with training data display the icon and label
  • Design review and signoff
  • Accessibility review and signoff
  • Po/PM review and signoff

Quality Assurance & Testing

  • I fixed|updated|added unit tests and integration tests for each feature (if applicable).
  • No sensitive information (i.e. PII/credentials/internal URLs/etc.) is captured in logging, hardcoded, or specs
  • Linting warnings have been addressed
  • Documentation has been updated (link to documentation *if necessary)
  • Screenshot of the developed feature is added
  • Accessibility testing has been performed

Error Handling

  • Browser console contains no warnings or errors.
  • Events are being sent to the appropriate logging solution
  • Feature/bug has a monitor built into Datadog or Grafana (if applicable)

Authentication

  • Did you login to a local build and verify all authenticated routes work as expected with a test user (non authed feature)

Requested Feedback

Will have to use review instance or local deployment of vets-website (Can't test on tugboat because of facility locator)

  1. Go to /find-locations
  2. Use address: 2414 Walbert Ave, Allentown, PA 18104
  3. Use Community providers for facility type
  4. Wait for box on right to allow typing
  5. Type in Podiatrist
  6. Click Search, look at results

If running local vets-website, you can use yarn watch --env api=https://dev-api.va.gov and then go to http://localhost:3001/find-locations if you have the MAPBOX_TOKEN in your .env

@eselkin eselkin marked this pull request as ready for review July 8, 2024 21:49
@eselkin eselkin requested review from a team as code owners July 8, 2024 21:49
@va-vfs-bot va-vfs-bot temporarily deployed to master/VACMS-15823-training-icons/main July 8, 2024 21:54 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to master/VACMS-15823-training-icons/main July 8, 2024 22:00 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to master/VACMS-15823-training-icons/main July 8, 2024 23:44 Inactive
Copy link

@laflannery laflannery left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eselkin We need to make sure that all content we are adding now is added in a way that that text will be resized properly for browser font settings - the typography updates the DST did allows for this sitewide but we need to make sure our individual products do as well. What this means is that <p> tags should be the default and <div> and <span> should be used less often.

For the Provider core training, currently you have everything wrapped in a <div> which has no default font size and will not resize according to browser settings. This screenshot is when I set everything to Very Large, notice how the provider name, direction link, etc are large but not "Provider core training":
image

By changing the wrapping tag to <p> this has a font size of 1.06rem and will scale accordingly. Now "Provider core training" is large when I change my font settings:
image

NOTE: You may notice that some other elements in the listings also don't scale (address, phone descriptions). This I had already noted in a separate ticket

Copy link
Contributor

@randimays randimays left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a few minor comments

@eselkin eselkin requested review from randimays and laflannery July 9, 2024 16:17
@va-vfs-bot va-vfs-bot temporarily deployed to master/VACMS-15823-training-icons/main July 9, 2024 16:38 Inactive
Copy link
Contributor

@rmessina1010 rmessina1010 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link

@laflannery laflannery left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

Copy link

@thejordanwood thejordanwood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving based on the screenshots.

Copy link
Contributor

@randimays randimays left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@eselkin eselkin merged commit 03ee7cc into main Jul 11, 2024
80 checks passed
@eselkin eselkin deleted the VACMS-15823-training-icons branch July 11, 2024 20:37
DLarson-Oddball pushed a commit that referenced this pull request Jul 17, 2024
* with class/color and training wording
* add testing for new componnent rendering inside other component
* non-semantic markup
* span
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants